Merged
Conversation
There's a problem right now for chunk eviction. In get_evict_targets we remove from the lru and modify the priority queue accordingly. We then take a subset of what has been modified and clean it after retrieving the relevant zones in get_clean_targets. The problem is the priority queue can now be potentially adjusted by reads that occur, meaning the priorities can be in an inconsistent state since we do not evict them immediately. Need to track if chunk is valid, maintain a array of list of chunks representing all zones. Chunks should be initialized to invalid initially. When a first write occurs it should be set to valid. When it's removed from the lru it should be set to invalid and we update the priority queue accordingly. If we ever do a read, but it is to an invalid chunk, that means we need to update the priority queue. This also means we do not need to remove from the lru in get_clean_targets, saving us a lot of time. We simply remove the retain, and then in get_evict_targets, when we are removing from the lru we check if it is an invalid entry and if so we remove it and skip it. So basically our lru may end up holding invalid items but that's fine.
2a46m4
approved these changes
Jan 2, 2026
Collaborator
2a46m4
left a comment
There was a problem hiding this comment.
looks good, just minor question
Owner
Author
|
Tested, working |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There's a problem right now for chunk eviction. In get_evict_targets we remove from the lru and modify the priority queue accordingly.
We then take a subset of what has been modified and clean it after retrieving the relevant zones in get_clean_targets.
The problem is the priority queue can now be potentially adjusted by reads that occur, meaning the priorities can be in an inconsistent state since we do not evict them immediately.
Need to track if chunk is valid, maintain a array of list of chunks representing all zones.
Chunks should be initialized to invalid initially.
When a first write occurs it should be set to valid.
When it's removed from the lru it should be set to invalid and we update the priority queue accordingly.
If we ever do a read, but it is to an invalid chunk, that means we need to update the priority queue.
This also means we do not need to remove from the lru in get_clean_targets, saving us a lot of time. We simply remove the retain, and then in get_evict_targets, when we are removing from the lru we check if it is an invalid entry and if so we remove it and skip it. So basically our lru may end up holding invalid items but that's fine.